Skip to content

feat(prefix): auto-tune prefix match cap from model max_model_len#1721

Open
mayur-tolexo wants to merge 1 commit into
llm-d:mainfrom
mayur-tolexo:feat-autotune-max-prefix-tokens
Open

feat(prefix): auto-tune prefix match cap from model max_model_len#1721
mayur-tolexo wants to merge 1 commit into
llm-d:mainfrom
mayur-tolexo:feat-autotune-max-prefix-tokens

Conversation

@mayur-tolexo

@mayur-tolexo mayur-tolexo commented Jun 23, 2026

Copy link
Copy Markdown

Closes #1505.

The prefix-cache producer caps matching at a fixed maxPrefixTokensToMatch (default 128k), which under-matches long prefixes on larger-context models and is one more thing to tune per model. Per the discussion on #1505, this auto-tunes it from the model's max_model_len when autoTune is on and the cap is left at its default. An explicitly set cap is still respected.

max_model_len isn't a metric, so it's read from /v1/models — fetched once per pod off the request path and cached, with concurrent misses collapsed to one fetch. Until it resolves, or if the pod can't be reached, we fall back to the current default, so nothing changes in the common case. The fetch reuses the model-server-metrics scheme/TLS flags, and cached entries are dropped when a pod leaves the active set.

Note: like the metrics scraper, the /v1/models request isn't authenticated — a follow-up if secured model servers are in scope.

$ go test -race -cover ./pkg/epp/framework/plugins/requestcontrol/dataproducer/approximateprefix/...
ok  	github.com/llm-d/llm-d-router/pkg/epp/framework/plugins/requestcontrol/dataproducer/approximateprefix	1.917s	coverage: 85.0% of statements

@mayur-tolexo
mayur-tolexo requested review from a team, liu-cong and vMaroon as code owners June 23, 2026 07:05
@github-actions github-actions Bot added the size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. label Jun 23, 2026
@github-actions

Copy link
Copy Markdown
Contributor

🚨 Unsigned commits detected! Please sign your commits.

For instructions on how to set up GPG/SSH signing and verify your commits, please see GitHub Documentation.

@mayur-tolexo
mayur-tolexo force-pushed the feat-autotune-max-prefix-tokens branch 2 times, most recently from 610b377 to d602688 Compare June 23, 2026 07:08
@vMaroon

vMaroon commented Jun 23, 2026

Copy link
Copy Markdown
Member

Thank you for the contribution! I think the goal is very aligned with where we wanna take the project.

max_model_len is constant per-endpoint runtime info, so I think it fits better as an endpoint attribute than a fetch+cache living inside this plugin.

The models-data-extractor already parses /v1/models into a per-endpoint ModelDataCollection attribute - it just doesn't keep max_model_len, though it's right there in the same payload. We could surface the field on that attribute and read it where the cap is derived, falling back to the default when it's absent.

That makes the models source a dependency of the cap derivation, so it should be declared as one. inflightload is the precedent.

@mayur-tolexo
mayur-tolexo force-pushed the feat-autotune-max-prefix-tokens branch from d602688 to b455c6a Compare June 23, 2026 10:36
@github-actions github-actions Bot added size/L Denotes a PR that changes 100-499 lines, ignoring generated files. and removed size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. labels Jun 23, 2026
@mayur-tolexo

Copy link
Copy Markdown
Author

Thank you for the contribution! I think the goal is very aligned with where we wanna take the project.

max_model_len is constant per-endpoint runtime info, so I think it fits better as an endpoint attribute than a fetch+cache living inside this plugin.

The models-data-extractor already parses /v1/models into a per-endpoint ModelDataCollection attribute - it just doesn't keep max_model_len, though it's right there in the same payload. We could surface the field on that attribute and read it where the cap is derived, falling back to the default when it's absent.

That makes the models source a dependency of the cap derivation, so it should be declared as one. inflightload is the precedent.

Thanks for the guidance - reworked it. max_model_len now rides the existing models attribute: added MaxModelLen to ModelData, and the prefix producer declares ModelsAttributeKey as a required dependency and reads it where the cap is derived, falling back to the default when absent. Dropped the standalone fetch+cache; PTAL.

@mayur-tolexo
mayur-tolexo force-pushed the feat-autotune-max-prefix-tokens branch 2 times, most recently from 6269f42 to d002e73 Compare June 23, 2026 11:07
@vMaroon

vMaroon commented Jun 24, 2026

Copy link
Copy Markdown
Member

Ok after further thinking - I think we can just go right away for the proper solution of creating an EndpointExtractor that runs once on endpoint-add instead of polling constant information. This would be aligned with this effort:

So it would be an EndpointExtractor on the EndpointNotificationSource fetching /v1/models once on endpoint-add and writing the same ModelsAttributeKey. Should also make it a dependency that self-wires as a default source.

The consumer stays as-is, only the source flips from the per-tick poll to a once-on-add fetch.

What do you think?

@mayur-tolexo

Copy link
Copy Markdown
Author

Ok after further thinking - I think we can just go right away for the proper solution of creating an EndpointExtractor that runs once on endpoint-add instead of polling constant information. This would be aligned with this effort:

So it would be an EndpointExtractor on the EndpointNotificationSource fetching /v1/models once on endpoint-add and writing the same ModelsAttributeKey. Should also make it a dependency that self-wires as a default source.

The consumer stays as-is, only the source flips from the per-tick poll to a once-on-add fetch.

What do you think?

+1, let's go with the extractor approach. Reading through #38147, will update the PR.

@vMaroon

vMaroon commented Jun 25, 2026

Copy link
Copy Markdown
Member

To clarify, the attached vLLM issue would be where we're heading, for the scope of this PR we'd still be only looking at /v1/models.

@mayur-tolexo
mayur-tolexo force-pushed the feat-autotune-max-prefix-tokens branch from d002e73 to cc813e5 Compare June 27, 2026 15:22
@github-actions github-actions Bot added size/XL Denotes a PR that changes 500-999 lines, ignoring generated files. and removed size/L Denotes a PR that changes 100-499 lines, ignoring generated files. labels Jun 27, 2026
@mayur-tolexo
mayur-tolexo force-pushed the feat-autotune-max-prefix-tokens branch from cc813e5 to 5d4c810 Compare June 27, 2026 15:33
The approximate-prefix-cache producer caps prefix matching at a fixed
maxPrefixTokensToMatch (default 128k). On models with a larger context
window that under-matches long shared prefixes, and it's one more knob
to set per model.

When autoTune is on and maxPrefixTokensToMatch is left at its default,
derive the cap from each endpoint's max_model_len instead, falling back
to the existing default when it isn't known. An explicitly configured
cap is always respected.

max_model_len rides the existing /v1/models attribute. Rather than poll
that endpoint on a timer for information that's fixed for an endpoint's
lifetime, a new models-endpoint-extractor fetches /v1/models once when an
endpoint is added and stores the model list (now carrying max_model_len)
as an attribute. It registers as the default producer of that attribute
and self-wires an endpoint-notification source, so the producer only
declares the dependency and reads the value. A failed or slow fetch
leaves the attribute unset and the cap falls back to the default.

Signed-off-by: Mayur Das <mayur.das@neevcloud.com>
@mayur-tolexo
mayur-tolexo force-pushed the feat-autotune-max-prefix-tokens branch from 5d4c810 to 735f527 Compare June 27, 2026 15:37
@github-actions

Copy link
Copy Markdown
Contributor

This PR is marked as stale after 21d of inactivity. After an additional 14d of inactivity (7d to become rotten, then 7d more), it will be closed. To prevent this PR from being closed, add a comment or remove the lifecycle/stale label.

@elevran

elevran commented Jul 20, 2026

Copy link
Copy Markdown
Collaborator

@mayur-tolexo @vMaroon - please close if not actively pursuing

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

size/XL Denotes a PR that changes 500-999 lines, ignoring generated files.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Auto-tune maxPrefixBlocksToMatch

3 participants